-
Notifications
You must be signed in to change notification settings - Fork 53
perfmon Metric names, descriptions, and tips #472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… deprecated. All metric descriptions in HTML report. Signed-off-by: Harper, Jason M <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modernizes metric naming to match perfmon standards by removing legacy names as the primary metric identifiers. Legacy names are now optional and deprecated, supporting backward compatibility through a new --legacy-names
flag. Additionally, metric descriptions are now included in HTML reports for enhanced usability.
- Updated metric naming system to use modern perfmon names by default
- Introduced optional
--legacy-names
flag for backward compatibility - Enhanced HTML reports with metric descriptions for better user experience
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
cmd/metrics/summary.go | Added metricDefinitions parameter to summary functions and updated template variable naming |
cmd/metrics/resources/perfmon/srf/srf.json | Removed LegacyName fields from metric definitions |
cmd/metrics/resources/perfmon/spr/spr.json | Removed LegacyName fields from metric definitions |
cmd/metrics/resources/perfmon/icx/icx.json | Removed LegacyName fields from metric definitions |
cmd/metrics/resources/perfmon/gnr/gnr.json | Removed LegacyName fields from metric definitions |
cmd/metrics/resources/perfmon/emr/emr.json | Removed LegacyName fields from metric definitions |
cmd/metrics/resources/base.html | Replaced hardcoded metric descriptions with dynamic template variable |
cmd/metrics/print.go | Added metric name translation functions and updated output format functions |
cmd/metrics/metrics.go | Added legacy names flag and updated function calls |
cmd/metrics/loader_perfmon.go | Updated metric lookups to use MetricName instead of LegacyName |
cmd/metrics/loader.go | Extended MetricDefinition struct with additional fields |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
* add Gaudi microarchitecture to report Signed-off-by: Harper, Jason M <[email protected]> * perfmon Metric names, descriptions, and tips (#472) * Metric names modernized to match perfmon. "Legacy" names optional and deprecated. All metric descriptions in HTML report. Signed-off-by: Harper, Jason M <[email protected]> * temporary threshold value Signed-off-by: Harper, Jason M <[email protected]> * tma metric thresholds Signed-off-by: Harper, Jason M <[email protected]> * style the tooltips Signed-off-by: Harper, Jason M <[email protected]> * indent on TMA child metrics Signed-off-by: Harper, Jason M <[email protected]> * add tuning tips Signed-off-by: Harper, Jason M <[email protected]> * skip threshold if no metric variables defined Signed-off-by: Harper, Jason M <[email protected]> * revert to legacy names Signed-off-by: Harper, Jason M <[email protected]> * set legacy name to name for legacy metric defs Signed-off-by: Harper, Jason M <[email protected]> * fix kernel_CPI metric formula and add threshold to kernel utilization Signed-off-by: Harper, Jason M <[email protected]> --------- Signed-off-by: Harper, Jason M <[email protected]> * add Gaudi microarchitecture to report Signed-off-by: Harper, Jason M <[email protected]> --------- Signed-off-by: Harper, Jason M <[email protected]>
This pull request introduces significant enhancements to the metrics loading and processing pipeline, primarily by adding support for threshold expressions and improving metric definition handling. The changes affect how metrics are parsed, transformed, and evaluated, with new fields and logic for threshold expressions, more robust filtering, and improved naming consistency.
Metric Definition and Threshold Support
MetricDefinition
struct to include fields forLegacyName
,Category
,Level
,ThresholdExpression
,ThresholdVariables
, andThresholdEvaluable
, enabling richer metric metadata and threshold evaluation. [1] [2]Perfmon Loader Improvements
MetricName
instead ofLegacyName
for more consistent identification, and improved error handling when metrics are missing. Added helper functions for finding metrics by both names. [1] [2] [3]Code Consistency and Usability
LegacyName
without themetric_
prefix for improved readability.Miscellaneous
These changes collectively make the metrics system more extensible and robust, especially in handling thresholds and ensuring consistent metric identification across loaders and output.